Skip to content

[Testing] Disable geoip downloader#25248

Merged
andrewkroh merged 2 commits intoelastic:masterfrom
andrewkroh:feature/disable-geoip-downloader
Apr 23, 2021
Merged

[Testing] Disable geoip downloader#25248
andrewkroh merged 2 commits intoelastic:masterfrom
andrewkroh:feature/disable-geoip-downloader

Conversation

@andrewkroh
Copy link
Copy Markdown
Member

@andrewkroh andrewkroh commented Apr 22, 2021

What does this PR do?

Disable database updates via an Elasticsearch property ingest.geoip.downloader.enabled: false. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

Closes: #25159

Why is it important?

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

@botelastic botelastic Bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 22, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 22, 2021 •
edited by jenkins-beats-ci Bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #25248 updated

  • Start Time: 2021-04-23T10:54:30.610+0000

  • Duration: 168 min 10 sec

  • Commit: b47c87d

Test stats 🧪

Test Results
Failed 0
Passed 47263
Skipped 5149
Total 52412

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 47263
Skipped 5149
Total 52412

@andrewkroh
Disable geoip downloader
37ed68f 
Disable database updates via an Elasticsearch property `ingest.geoip.downloader.enabled: false`. This will cause Elasticsearch to use the circa 2019 maxmind database version that is embedded. This would work for now, but the embedded database will be removed in 8.x IIUC.

This prevents our golden file tests from failing everytime the database changes geo or asn values. It also prevents race conditions in tests that might run before the database has been updated or in cases where the downloader service is unavailable.

Closes: elastic#25159
@andrewkroh andrewkroh force-pushed the feature/disable-geoip-downloader branch from fad98c3 to 37ed68f Compare April 22, 2021 19:49
@andrewkroh andrewkroh added the Filebeat Filebeat label Apr 22, 2021
@andrewkroh andrewkroh marked this pull request as ready for review April 22, 2021 23:00
@andrewkroh
Copy link
Copy Markdown
Member Author

andrewkroh commented Apr 23, 2021

The generator tests are failing. I opened a separate fix for them in #25251.

@andrewkroh andrewkroh added Team:Elastic-Agent Label for the Agent team Team:Integrations Label for the Integrations team Team:Security-External Integrations labels Apr 23, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 23, 2021

Pinging @elastic/integrations (Team:Integrations)

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 23, 2021

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 23, 2021

Pinging @elastic/agent (Team:Agent)

@botelastic botelastic Bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Apr 23, 2021
@andrewkroh
Copy link
Copy Markdown
Member Author

andrewkroh commented Apr 23, 2021

This fix needs backported to 7.x and 7.13. AFAIK neither of those branches have had their golden files updated to work on the new geoip database so those PRs should be much smaller.

@exekias
Copy link
Copy Markdown
Contributor

exekias commented Apr 23, 2021

+1 to this, do you know how often the actual fields change? or perhaps that doesn't happen at all? I'm wondering if we need a way to detect that situation to update mappings

@andrewkroh
Copy link
Copy Markdown
Member Author

andrewkroh commented Apr 23, 2021

Field mapping changes would only change when Elasticsearch modifies the geoip processor. The geoip processor interacts with the raw database data so it should isolate us from DB schema changes should they be implemented by MaxMind. And according to MaxMind the database data updates ~weekly.

@andrewkroh
Copy link
Copy Markdown
Member Author

andrewkroh commented Apr 23, 2021

run tests

@andrewkroh andrewkroh mentioned this pull request Apr 23, 2021
Merged
3 tasks
@mtojek mtojek mentioned this pull request Apr 23, 2021
Merged
jsoriano
jsoriano approved these changes Apr 23, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@andrewkroh andrewkroh merged commit 61a34ae into elastic:master Apr 23, 2021
@andrewkroh andrewkroh mentioned this pull request Apr 23, 2021
Merged
3 tasks
@jsoriano jsoriano mentioned this pull request Oct 27, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsoriano jsoriano jsoriano approved these changes

Assignees

No one assigned

Labels

Filebeat Filebeat review Team:Elastic-Agent Label for the Agent team Team:Integrations Label for the Integrations team :Testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Discuss] Upstream geoip database changes affecting tests

4 participants

@andrewkroh @elasticmachine @exekias @jsoriano